Use pinned commit PyTorch on CI instead of nightly #6564
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/6564
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit f3ce7ef with merge base 1f38016 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
| @@ -1,5 +1,5 @@ | |||
| mpmath==1.3.0 | |||
| numpy==1.22.0; python_version == '3.10' | |||
| numpy==1.21.3; python_version == '3.10' | |||
There was a problem hiding this comment.
Good catch. Thanks for making it match pyproject.toml https://github.com/pytorch/executorch/blob/main/pyproject.toml#L58-L60
install_requirements.py
Outdated
| if USE_PYTORCH_NIGHTLY: | ||
| # pip packages needed by exir. | ||
| EXIR_REQUIREMENTS = [ | ||
| f"torch==2.6.0.{NIGHTLY_VERSION}", |
There was a problem hiding this comment.
| f"torch==2.6.0.{NIGHTLY_VERSION}", | |
| f"torch==2.6.0.{NIGHTLY_VERSION}" if USE_PYTORCH_NIGHTLY else "torch", |
Maybe this is cleaner, so you don't have to specify other packages in both branches?
There was a problem hiding this comment.
Yeah, good idea, that looks nicer indeed
| # The pip repository that hosts nightly torch packages. | ||
| TORCH_NIGHTLY_URL = "https://download.pytorch.org/whl/nightly/cpu" | ||
|
|
||
| # pip packages needed by exir. |
There was a problem hiding this comment.
Can you keep the comment for why the version is not needed when fetching nightly here?
There was a problem hiding this comment.
Oh I thought that I have mention it in the argument section above. But I could add the comment back if you think it's closed to the code
After landing pytorch/executorch#6564, we need to update the pinned ExecuTorch commit on PyTorch is fix the regression on PyTorch side. The change to `.ci/docker/common/install_executorch.sh` is needed because it's how the dependencies are setup on ExecuTorch CI now. Pull Request resolved: #139700 Approved by: https://github.com/larryliu0820, https://github.com/malfet
After landing pytorch/executorch#6564, we need to update the pinned ExecuTorch commit on PyTorch is fix the regression on PyTorch side. The change to `.ci/docker/common/install_executorch.sh` is needed because it's how the dependencies are setup on ExecuTorch CI now. Pull Request resolved: pytorch#139700 Approved by: https://github.com/larryliu0820, https://github.com/malfet
I think this is a regression. CI needs to use PyTorch built from the pinned commit instead of installing nightly (using
install_requirements.sh). Otherwise, both PT and ET CI won't be able to catch regression against their latest commits.Various examples:
I attempt to fix the issue by instroducing
--use-pt-pinned-committoinstall_requirements.sh, so that it can keep the current behavior of installing pinned PT nightly when running locally while use pinned PT commit on CI. Note that the Docker image only has the pinned PT commit installed.Testing
The correct PyTorch built from the pinned commit (23d590e518688f96e1d1947a08e9ca27df3e67e4) is used https://github.com/pytorch/executorch/actions/runs/11601294150/job/32303693643#step:12:6496
On the other hand, using nightly looks like https://github.com/pytorch/executorch/actions/runs/11600922135/job/32302497353#step:12:6552